-
Notifications
You must be signed in to change notification settings - Fork 15
Add parameter to factory method for setting handler options #246
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
Depending on when #245 is merged, this PR will need to contain a version bump and a changelog entry.
Added a comment.
Also, did you start working on unit tests for that? Did you run into any issues with those?
Added tests to the PR. As mentioned in #233 , I cannot run the tests locally unless I remove the analyzer reference from the .csproj file. |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes!
since 1.4.0 has been published, we'll need an entry in the changelog (today, minor) and a version bump in the csproj to expedite the release of those changes.
Microsoft.Kiota.Http.HttpClientLibrary.Tests/KiotaClientFactoryTests.cs
Outdated
Show resolved
Hide resolved
No need to expedite. Thanks tho. |
@pschaeflein by expedite I meant "we don't want to have to open another PR just for the version bump because we're lazy" 🤣 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
closes #233